Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Node 10 to Travis config and remove Node 6 #4383

Merged
merged 9 commits into from
Jul 16, 2018

Conversation

iansu
Copy link
Contributor

@iansu iansu commented Apr 30, 2018

Update Travis config to add support for Node 10 and remove Node 6.

@iansu
Copy link
Contributor Author

iansu commented Apr 30, 2018

Appveyor config also needs to be updated but I'm not sure if they support Node 10 yet. Actually I'm not sure if Travis does either so let's see what happens. 😀🍿

@DanielRuf
Copy link

DanielRuf commented Jun 7, 2018

Both support it.

Can you update/rebase the PR and trigger a new build?

@gaearon
Copy link
Contributor

gaearon commented Jun 7, 2018

Do we plan to remove support for Node 6?

@iansu
Copy link
Contributor Author

iansu commented Jun 7, 2018

@gaearon @Timer and I discussed dropping support for Node 6. It's in maintenance now. EOL is April 2019.

@iansu
Copy link
Contributor Author

iansu commented Jun 7, 2018

I will merge some of our recent test improvements into this branch which will re-trigger the tests.

@gaearon
Copy link
Contributor

gaearon commented Jun 7, 2018

Cool. I'm cool with dropping it

@Timer
Copy link
Contributor

Timer commented Jun 8, 2018

For context, I believe we dropped Node 4 when it entered maintenance (LTS expired), but before EOL.

Normally I'd want to wait for EOL, but the difference in features and speed between 6/8 is so significant I don't think it's worth keeping around until EOL. :)

@iansu iansu force-pushed the update-travis-config branch from 5fbd6d5 to 0b1ae21 Compare June 10, 2018 19:09
@Timer Timer closed this in #4626 Jun 18, 2018
@Timer Timer reopened this Jun 18, 2018
.travis.yml Outdated
@@ -4,6 +4,7 @@ language: node_js
node_js:
- 8
- 9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove Node 9 and merge this with next please?

@iansu iansu force-pushed the update-travis-config branch from 51df705 to 28032f2 Compare June 18, 2018 17:59
@iansu
Copy link
Contributor Author

iansu commented Jun 18, 2018

Ugh. Tests passed on Node 10 for simple and monorepo but failed for install and kitchensink. They just timed out. For the simple test suite it took twice as long to run on Node 10 as it did on Node 8. Still want me to merge this?

@Timer
Copy link
Contributor

Timer commented Jun 18, 2018

I want to get a PR passing before we merge ... what in the world is going on. 😅

@iansu
Copy link
Contributor Author

iansu commented Jun 18, 2018

I don't know but it's a real mess. Sometimes Travis fails, sometimes Appveyor fails, sometimes they both fail and every once in a while they both miraculously pass. Often they will go from passing to failing and the only difference is a change in the README. 🤷‍♂️

@DanielRuf
Copy link

Sounds like some race conditions.

@Timer
Copy link
Contributor

Timer commented Jun 18, 2018

Can you add --timeout 30000 to the mocha calls?

@iansu
Copy link
Contributor Author

iansu commented Jun 19, 2018

This seems like a bit of an improvement. Kitchensink is very flaky on both Node 8 and 10. It passed on Travis but only because I re-ran it 4 times. It seems to fail in different places with different errors.

Installs is consistently failing on Node 10 at the same place on Travis and Appveyor, although it fails in different ways on both. This could be due to the different version of yarn.

This is what the logs look like on Travis and it just hangs here for 30+ minutes until the test times out:

+npx create-react-app --scripts-version=/tmp/tmp.hATqpWFGgY/enoah-scripts-0.9.0.tgz test-app-scoped-fork-tgz
npx: installed 289 in 14.817s
Creating a new React app in /tmp/tmp.hATqpWFGgY/test-app-scoped-fork-tgz.
Installing packages. This might take a couple of minutes.
Installing react, react-dom, and @enoah_netzach/react-scripts...
yarn add v1.3.2
(node:4567) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
info No lockfile found.
[1/4] Resolving packages...

On Appveyor it actually fails on the same part and this is what the logs look like there:

+ npx create-react-app --scripts-version=/tmp/tmp.MzXAX6CkKW/enoah-scripts-0.9.0.tgz test-app-scoped-fork-tgz
npx: installed 289 in 8.779s
Creating a new React app in C:\Users\appveyor\AppData\Local\Temp\1\tmp.MzXAX6CkKW\test-app-scoped-fork-tgz.
Installing packages. This might take a couple of minutes.
Installing react, react-dom, and @enoah_netzach/react-scripts...
yarn add v1.5.1
(node:504) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
info No lockfile found.
[1/4] Resolving packages...
Aborting installation.
Unexpected error. Please report it as a bug:
{ Error: Cannot find module 'C:\Users\appveyor\AppData\Local\Temp\1\tmp.MzXAX6CkKW\test-app-scoped-fork-tgz\node_modules\@enoah_netzach\react-scripts\package.json'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:594:15)
    at Function.Module._load (internal/modules/cjs/loader.js:520:25)
    at Module.require (internal/modules/cjs/loader.js:650:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at checkNodeVersion (C:\Users\appveyor\AppData\Roaming\npm-cache\_npx\2372\node_modules\create-react-app\createReactApp.js:544:23)
    at getPackageName.then.then.then.packageName (C:\Users\appveyor\AppData\Roaming\npm-cache\_npx\2372\node_modules\create-react-app\createReactApp.js:345:7)
    at process._tickCallback (internal/process/next_tick.js:68:7) code: 'MODULE_NOT_FOUND' }
Deleting generated file... package.json
Deleting test-app-scoped-fork-tgz/ from C:\Users\appveyor\AppData\Local\Temp\1\tmp.MzXAX6CkKW
Done.
++ set +x
e2e-installs.sh: ERROR! An error was encountered executing line 200.
Cleaning up.
yarn config v1.5.1
(node:444) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
success Set "registry" to "https://registry.yarnpkg.com".
Done in 0.05s.
Exiting with error.
Command exited with code 1

@Timer
Copy link
Contributor

Timer commented Jun 19, 2018

Can we install the latest Yarn?

@iansu
Copy link
Contributor Author

iansu commented Jul 14, 2018

I added a before_install script to install the latest version of Yarn on Travis. I'm not sure if it made a difference though. The tests did pass but I had to rerun a couple of them. The old-node test failed because it tried to install Yarn with Node 0.10 which doesn't work.

I'm trying to figure out if I can skip installing Yarn with Node 0.10. Alternatively we could update our old-node test to use Node 4 or 6.

@iansu iansu force-pushed the update-travis-config branch from ee5e58b to ac7031f Compare July 15, 2018 18:27
@iansu
Copy link
Contributor Author

iansu commented Jul 15, 2018

Looks like all the tests are now passing on Travis. I had to restart two of them because they failed to install the latest version of Yarn. It looked like a temporary issue with the Yarn site unrelated to our tests.

Appveyor tests are queued...

🍿

@iansu
Copy link
Contributor Author

iansu commented Jul 15, 2018

Appveyor!!

@iansu iansu closed this Jul 15, 2018
@iansu iansu reopened this Jul 15, 2018
@iansu iansu force-pushed the update-travis-config branch 2 times, most recently from a91262f to c5bb386 Compare July 15, 2018 23:57
@iansu iansu force-pushed the update-travis-config branch from c5bb386 to 9828d6c Compare July 16, 2018 02:27
@iansu
Copy link
Contributor Author

iansu commented Jul 16, 2018

It works! 🎉

Updating Yarn seems to make a difference with Appveyor. On Travis the process of updating Yarn itself seems to be a bit flaky. The download failed a couple of times and there was a checksum error another couple of times. I'm not sure if this is an issue with the Yarn download site or with Travis. Either way, this seems like a big improvement.

@iansu iansu merged commit 92d9c5a into facebook:next Jul 16, 2018
@andresroberto
Copy link

Should the readme be updated to reflect the drop of Node 6? Right now it states:

You’ll need to have Node >= 6 on your local development machine

@iansu
Copy link
Contributor Author

iansu commented Aug 1, 2018

The README is based on CRA 1.0. We will be updating all the documentation before 2.0 is released.

@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
@iansu iansu deleted the update-travis-config branch October 18, 2019 05:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants